feat(bull_ui): design-system package + component catalogue - #2302

Claude finished @ethicnology's task in 3m 21s —— View job

Claude PR review

• Gather context (diff, AGENTS.md, CI workflow, key files)
• Review theme/tokens (bull_theme.dart, bull_tokens.dart, app_theme.dart)
• Review components for correctness bugs & regressions
• Review catalogue + build wiring (makefile, pubspec, CI)
• Post findings + summary

Reviewed for correctness bugs, security issues, and regressions. This is a clean, well-documented, almost entirely additive PR: a new packages/bull_ui design-system package + a dev-only Widgetbook catalogue. The only change to existing app code is lib/core/themes/app_theme.dart, which is purely additive. No BB* originals are touched. Overall quality is high. I found no security issues and no app-runtime regressions — the findings below are mostly about CI wiring and a couple of behavioural nits.

🟠 Findings

1. New package tests don't run in CI (test-rot risk) — makefile:190-192 + .github/workflows/analyze_and_test.yml
CI's test job runs make unit-test, which is fvm flutter test test/ — the root package only. The 14 new packages/bull_ui widget tests and the catalogue test are never executed by CI (make test / make unit-test don't descend into workspace members). make analyze does cover them (single analyzer context across the pub workspace, as AGENTS.md predicts), but the tests themselves can silently break and CI will stay green. Since the whole point of the 14 tests is to guard Bull* behaviour, I'd wire them in now — e.g. a melos test target (fvm dart run melos test) or an explicit per-package fvm flutter test — so the safety net is actually load-bearing. Fix this →

🟡 Minor / confirm-intent

2. BullButton wraps every button in a Tooltip(message: label) — packages/bull_ui/lib/src/buttons/bull_button.dart:156-166
This appears to be new vs the core/widgets/buttons/button.dart original. Two consequences worth confirming: (a) long-pressing any button now shows a tooltip app-wide once these are adopted, and (b) for an icon-only button (label == '', line 136) the tooltip message is an empty string, so a long-press shows an empty bubble. Consider message: label.isEmpty ? null : label if that's unintended.

3. BullSegmented throws on an empty items set — packages/bull_ui/lib/src/controls/bull_segmented.dart:39,50
widget.initialValue ?? widget.items.first (and the same in build) will throw StateError: No element if items is empty. Also, the disabledItems.contains(v) early-return in onValueChanged (line 52) updates neither selectedSegment nor the parent, but CustomSlidingSegmentedControl has already slid its thumb to the disabled segment — so a disabled tap can leave the thumb visually on a value that was rejected. Both likely mirror the original; flagging since this control is called out as a UTXO sort/filter primitive.

4. Style divergence from the original (intentional?) — packages/bull_ui/lib/src/inputs/bull_input_text.dart:177
The port uses textTheme.headlineMedium where the original BBInputText uses headlineSmall (lib/core/widgets/inputs/text_input.dart:129). If Bull* is meant to be a faithful 1:1 port at this stage, this is a silent visual change; if it's a deliberate go-forward tweak, ignore. (Note: the Material character counter from maxLength is present in both, so that's not a regression.)

✅ Verified OK

• app_theme.dart — confirmed all 39 fields referenced by _bullThemeFrom exist on AppColors (light & dark) in lib/core/themes/colors.dart, so the new extensions: [...] won't fail to compile or throw at theme-build time. BullThemeX.bull uses ! and will throw if a consumer renders outside a tree carrying the extension — acceptable and documented.
• Reproducible-build claim holds — bull_ui is now a real app dependency, but its transitive deps (gap, auto_size_text, custom_sliding_segmented_control, shimmer) are already direct app deps, so no new packages enter the APK. widgetbook* lands only in the shared pubspec.lock, not in the app's dependency graph.
• BullTheme.copyWith/lerp cover every field; no missed members.
• BullCountdown recomputes against wall-clock and cancels its timer in dispose(); BullSnackBar static overlay/timer lifecycle guards against unmounted access.
• Clipboard reads/writes (bull_paste_input.dart, bull_details_table.dart) are standard and safe.

Nice groundwork for #760. The architecture note in the PR body (sanctioning Flutter-UI packages under packages/ in ARCHITECTURE.md/AGENTS.md) is worth landing soon so the next package doesn't re-litigate it.
· bull_ui_and_catalogue

@claude is it better now ?

Claude finished @ethicnology's task in 1m 13s —— View job

Re: "is it better now?" — yes, mostly ✅

I checked commit fc9a75dc4 against my four earlier findings. Three are cleanly resolved; the fourth (CI) is fixed in spirit but has one subtle gap.

• Finding 2 — empty tooltip → fixed. bull_button.dart:159 now early-returns button when overlay == null || label.isEmpty, so icon-only buttons no longer show an empty bubble. 👍
• Finding 4 — input text role → fixed. bull_input_text.dart:177 now uses headlineSmall, matching BBInputText (1:1 port restored).
• Finding 3 — empty BullSegmented → guarded. The new assert(items.length > 0, …) turns the silent StateError into an explicit contract. Reasonable. (The secondary note — a disabled tap leaving the thumb slid onto a rejected value — is untouched, but that was a "confirm-intent" item that mirrors the original, so fine to defer.)
• [~] Finding 1 — tests in CI → wired, but the loop swallows failures.

🟠 One thing to tighten on the CI fix

makefile:193-198:

@for p in packages/*/; do \
    if [ -d "$${p}test" ]; then \
        echo "🏃 running $${p}test"; \
        ( cd "$$p" && fvm flutter test --reporter=compact ); \
    fi; \
done

The whole for runs in one shell, and its exit status is just the last iteration's. So if bull_ui tests fail but bull_ui_catalogue (alphabetically later) passes, the loop exits 0 and make unit-test stays green — the failure is masked. That's the exact "CI stays green while tests break" risk finding #1 was meant to close, so it's worth propagating the error:

@for p in packages/*/; do \
    if [ -d "$${p}test" ]; then \
        echo "🏃 running $${p}test"; \
        ( cd "$$p" && fvm flutter test --reporter=compact ) || exit 1; \
    fi; \
done

(or @set -e; for p in …). Fix this →

Everything else from the prior review still stands as ✅. Once the loop propagates failures, the safety net is fully load-bearing. Nice iteration.
· bull_ui_and_catalogue